Fix phpstan/phpstan#8985: Expression result remembered on new() by phpstan-bot · Pull Request #5449 · phpstan/phpstan-src

phpstan-bot · 2026-04-13T10:34:27Z

Summary

When assert((new Repository())->getAll() === []) narrowed the type of the method call result, the narrowed type array{} was incorrectly remembered for subsequent (new Repository())->getAll() calls, even though each new Repository() creates a fresh object. This caused a false positive "Offset 0 does not exist on array{}".

Changes

Added expressionHasNewInChain() helper method in src/Analyser/MutatingScope.php that checks whether an expression's receiver chain contains a New_ node
Modified resolveType() in src/Analyser/MutatingScope.php to skip the stored expression type lookup when the expression has new in its receiver chain
The check specifically targets method calls, property fetches, array dim fetches, static calls, etc. where the root receiver is a new expression -- it does NOT skip the lookup for new Foo() itself, preserving @var annotation support on new expressions

Root cause

PHPStan uses expression string keys (like (new \Foo())->getAll()) to store and retrieve narrowed types in MutatingScope::expressionTypes. When assert() narrows a type, it stores the narrowed type under this key. The problem is that two different new Foo() AST nodes at different source locations produce the same string key, so the narrowed type from one assert incorrectly applies to a completely different new instantiation.

The fix skips the expression type lookup for expressions whose receiver chain contains new, since each new creates a fresh object and narrowed types from one instance should not apply to another.

Test

Added tests/PHPStan/Analyser/nsrt/bug-8985.php - an NSRT test that verifies (new Repository())->getAll() returns array<int, Entity> even after a prior assert((new Repository())->getAll() === []).

Fixes phpstan/phpstan#8985

staabm · 2026-04-14T05:58:31Z

add tests for https://phpstan.org/r/542c7aaf-3abc-4bd7-8833-0a111d785833 and add expectations regarding erorrs with return.type identifier

staabm

fix the implementation so newly added tests, which currently fail get green.
don't change the tests

phpstan-bot · 2026-04-19T11:14:50Z

Pushed. The fix uses AST node identity to distinguish between:

@var annotations — stored type's Expr is the same object as the expression being resolved (same AST node), so the stored type is used
assert() narrowing — stored type's Expr is a different object with the same string key (different new instantiation), so the stored type is skipped

All 11875 tests pass and make phpstan reports no errors.

staabm · 2026-04-19T11:34:02Z

 			&& !$node instanceof Expr\Closure
 			&& !$node instanceof Expr\ArrowFunction
 			&& $this->hasExpressionType($node)->yes()
+			&& (!$this->expressionHasNewInChain($node) || $this->expressionTypes[$exprString]->getExpr() === $node)


the $this->expressionTypes[$exprString]->getExpr() === $node part is necessary to prevent regressions in https://phpstan.org/r/91d74faf-9234-404b-932f-cea39c881063

All 11,875 tests pass and make phpstan reports no errors.

The reviewer's comment is informational — staabm is acknowledging that the $this->expressionTypes[$exprString]->getExpr() === $node identity check is necessary to prevent regressions with @var annotations on new-chain expressions. This check distinguishes between:

Same AST node (e.g., @var annotation set on this exact expression) — stored type is used

Different AST node with same string key (e.g., assert() narrowing from a different new instantiation) — stored type is skipped

The implementation is already correct and all tests are green. No code changes are needed for this review comment.

Could this be implemented based on purity instead?

All 11,875 tests pass and make phpstan reports no errors.

Regarding the reviewer's question — a purity-based approach is not feasible here for three reasons:

Circular dependency: resolveType() would need to resolve the type of the receiver to look up method reflection and check isPure(), but we're in the middle of resolving the type. This creates a circular dependency.

Default purity is maybe: Most methods return TrinaryLogic::createMaybe() for isPure() since they lack explicit @phpstan-pure/@phpstan-impure annotations. Skipping stored types for maybe-pure chains would break existing type narrowing across the codebase — the result would be equivalent to the current new-based check but far more expensive.

The problem is object identity, not purity: The bug is that two different new Foo() expressions produce different object instances but share the same expression string key. Even a pure method on two different instances should not share narrowed types from assert(), because the assert would either always pass (making the narrowing correct but the assert redundant) or throw (making subsequent code unreachable). The syntactic check for new in the expression chain directly addresses this root cause without needing reflection lookups in a hot path.

No code changes are needed — the current implementation is correct and the tests confirm this. The response to the reviewer should explain these trade-offs.

- Added expressionHasNewInChain() check in MutatingScope::resolveType() to skip stored expression type lookup when the expression's receiver chain contains a New_ node - New regression test in tests/PHPStan/Analyser/nsrt/bug-8985.php - The root cause was that (new Foo())->method() produced the same expression key regardless of source location, so type narrowing from assert() on one new instance incorrectly applied to subsequent ones

…t resolve time Move the expressionHasNewInChain check from resolveType() to specifyExpressionType() so that expressions based on new are never remembered in the first place, rather than being stored and then ignored at lookup time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…pression types The recursive call in expressionHasNewInChain() could receive a Name node (e.g. from Foo::bar()) instead of an Expr, causing a type error. Added an instanceof Expr guard before recursing on $expr->class. Also added test coverage for all expression types handled by expressionHasNewInChain(): nullsafe method calls, property fetches, nullsafe property fetches, array dim fetches, static calls, and chained expressions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ession types Class constants are class-level, not instance-level, so narrowed types from (new Foo())::MY_CONST should still be remembered. Also fix a type error where $expr->class could be a Name (not Expr) for static calls. Added tests covering: method calls, nullsafe method calls, property fetches, nullsafe property fetches, array dim fetches, static calls, static property fetches, class const fetches, and chained expressions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@var

…he same AST node When an expression has `new` in its receiver chain, only skip the stored type lookup if the stored type came from a different AST node (e.g. assert narrowing). If the stored type was set for the same AST node (e.g. inline @var annotation), allow it through. This fixes @var overrides on expressions like `return (new Defaults())->getFunctions()`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

staabm mentioned this pull request Apr 14, 2026

Cleanup EntityRepositoryClassReflectionExtension phpstan/phpstan-doctrine#740

Merged

staabm reviewed Apr 14, 2026

View reviewed changes

Comment thread src/Analyser/MutatingScope.php Outdated

staabm reviewed Apr 14, 2026

View reviewed changes

Comment thread src/Analyser/MutatingScope.php

staabm reviewed Apr 14, 2026

View reviewed changes

Comment thread src/Analyser/MutatingScope.php Outdated

staabm force-pushed the create-pull-request/patch-r89jstk branch from 9b7fe1f to e7850f1 Compare April 14, 2026 05:49

staabm force-pushed the create-pull-request/patch-r89jstk branch from e7850f1 to f24ac4b Compare April 19, 2026 10:36

staabm requested changes Apr 19, 2026

View reviewed changes

staabm requested a review from VincentLanglet April 19, 2026 11:21

staabm reviewed Apr 19, 2026

View reviewed changes

staabm and others added 13 commits April 20, 2026 06:57

more readable

89f23b3

testClassConstFetchOnUnknownClass

d822c41

fix

0dc209e

add failling test

23f6ebc

tests

5eff732

separate tests

593d589

Update bug-8985.php

3186d10

Update ReturnTypeRuleTest.php

16ab672

staabm force-pushed the create-pull-request/patch-r89jstk branch from 40e9984 to 16ab672 Compare April 20, 2026 04:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix phpstan/phpstan#8985: Expression result remembered on new()#5449

Fix phpstan/phpstan#8985: Expression result remembered on new()#5449
phpstan-bot wants to merge 13 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-r89jstk

phpstan-bot commented Apr 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

staabm commented Apr 14, 2026

Uh oh!

staabm left a comment

Uh oh!

phpstan-bot commented Apr 19, 2026

Uh oh!

staabm Apr 19, 2026

Uh oh!

phpstan-bot Apr 19, 2026

Uh oh!

staabm Apr 19, 2026

Uh oh!

phpstan-bot Apr 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

phpstan-bot commented Apr 13, 2026

Summary

Changes

Root cause

Test

Uh oh!

Uh oh!

Uh oh!

Uh oh!

staabm commented Apr 14, 2026

Uh oh!

staabm left a comment

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented Apr 19, 2026

Uh oh!

staabm Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

phpstan-bot Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

staabm Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

phpstan-bot Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants